Skip to content

Bugfix RFLINK remove group#6580

Merged
balloob merged 5 commits into
home-assistant:devfrom
pvizeli:fix_rflink_light
Mar 16, 2017
Merged

Bugfix RFLINK remove group#6580
balloob merged 5 commits into
home-assistant:devfrom
pvizeli:fix_rflink_light

Conversation

@pvizeli
Copy link
Copy Markdown
Member

@pvizeli pvizeli commented Mar 13, 2017

Description:

Remove Group hack from platforms. It is the only platform that do that with lutron. With new bootstrap it is not anymore supported. I try first a hack with set entity_id on it but that will not work.

We have a nice group editor and I don't think it is the task of platform to create groups. A platform should only setup the device to entity and don't things inside core. Lutron is the best example that it make to complex and lot of hacks for self grouping.

Related issue (if applicable): fixes #6571

@mention-bot
Copy link
Copy Markdown

@pvizeli, thanks for your PR! By analyzing the history of the files in this pull request, we identified @fabaff and @aequitas to be potential reviewers.

@pvizeli pvizeli added this to the 0.40.1 milestone Mar 13, 2017
@aequitas
Copy link
Copy Markdown
Contributor

Thanks for jumping on this. I didn't have time to investigate it yet.

However dropping 'automatic group adding' as a feature completely is the wrong direction for this component.

The nature of these devices can cause a lot unwanted entries to appear if automatic detection is enabled (rflink devices from neighbors mostly). And not everyone might like to have to manually add all their devices to config (or have to change config/restart to detect new devices).

What would be the best solution in line with best practices according to you?

@pvizeli
Copy link
Copy Markdown
Member Author

pvizeli commented Mar 13, 2017

A reboot are not needed for group changes and now it could be done on UI.

Devices are change not every day. So you need setup your stuff once and not setup every start. So it is not need on platform level too

@aequitas
Copy link
Copy Markdown
Contributor

Makes sense.

@aequitas
Copy link
Copy Markdown
Contributor

@pvizeli consider bundling this feature with the bugfix: pvizeli#1

I had it planned already to bring rflink inline with other components. If its not included users won't be able to prevent their homepage being flooded with unwanted devices.

@aequitas aequitas mentioned this pull request Mar 13, 2017
@balloob balloob modified the milestone: 0.40.1 Mar 16, 2017
@balloob balloob merged commit 774fd19 into home-assistant:dev Mar 16, 2017
balloob pushed a commit that referenced this pull request Mar 16, 2017
* Bugfix RFLINK remove group

* Remove group hack from lutron too

* fix tests

* fix lint

* fix lint
@balloob balloob mentioned this pull request Mar 16, 2017
@thecynic
Copy link
Copy Markdown
Contributor

thecynic commented Mar 22, 2017

This PR broke the Lutron component. I wish I had a change to review and test.

File "/home/pi/home-assistant/homeassistant/helpers/entity.py", line 204, in async_update_ha_state
 "No entity id specified for entity {}".format(self.name))
homeassistant.exceptions.NoEntitySpecifiedError: No entity id specified for entity Exterior Sconces

Seems like you removed setting entity_id by only setting object_id, which isn't enough.

By the way, I disagree with the notion about auto-grouping. The way Lutron software works it effectively forces you to organize the devices into rooms (areas) and places in the room. This is already, thus, forcing you to create groups. That's also how it shows up in the Lutron software, and is self described by the XML file we parse on startup. Dropping that structure on the floor and forcing the user to recreate it seems counter-productive. If you want to add an option to turn-off the auto-grouping for those who want to go the manual route that's fine, but getting rid of this feature seems wrong.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Rflink: 0.40 broke entities with space in the name

7 participants